fix(knowledge): rank session-start injection by relevance, cap per entity, disclose the rest - #1894

Summary

Session-start knowledge injection selected observations with:

WHERE o.is_active = 1 AND o.confidence >= ?
ORDER BY e.name, o.last_confirmed_at DESC
LIMIT ?

ORDER BY e.name is alphabetical. Entity name has no relationship to how useful an
observation is, so the LIMIT was a filter on spelling. In production all 50 slots went
to AccountMap..AgentReliability, 46 of them to the single AgentBehavior grab-bag —
while the very same payload instructed the agent to consult ContentStyle, CodeQuality,
User, Architecture and BusinessStrategy before making decisions. Every one of those
sorts after "A". None had ever been injected, and nothing in the payload hinted they
existed, so an agent had no reason to search for them.

This is a quality bug as much as a token bug: the instructions told the agent to use
knowledge the same response structurally guaranteed it would never see.

R3 of the token-optimization program. Follows R1 (#1891).

What changed

• Rank by the formula that already exists (rules 24/59, not a second one).
  computeRelevanceScore — confidence × 1/(1 + age/30d) on last confirmation, so
  confirm_knowledge restores rank — extracted as the canonical JS definition, mirrored
  into SQL, and pinned by a parity test.
• Per-entity cap via ROW_NUMBER() OVER (PARTITION BY entity_id), so one sprawling
  entity cannot crowd out every other topic.
• now is a bound parameter, never strftime('now'), and ties break to a total order
  (score DESC, last_confirmed_at DESC, id ASC) — identical inputs give identical output.
• A complete entity index appended to knowledgeDirectives, disclosing what was not
  injected and naming the tool that retrieves it. Returns {entries, totalEntities} so a
  truncated index can never be labelled "full".
• The three DO reads run as an isolated Promise.allSettled fan-out — if ranked
  retrieval fails, the index alone still tells the agent what exists, instead of the
  pre-existing behaviour of silently injecting nothing.
• Both limits env-configurable (Principle XI); per-row fault isolation on both reads (rule 50).

Post-Mortem

What broke. Injection was ordered by a key uncorrelated with its purpose, and the
truncation was silent. Either alone is a bug; together they are undetectable from the
inside — every component "works", the payload is well-formed, the tests pass. It was found
by measurement, not by failure, and had been stable and systematically biased for months.

Class of bug. A cap whose ordering key is uncorrelated with the cap's purpose, with no
disclosure of what was excluded. It bites hardest when the consumer is an LLM, because a
model cannot notice an absence — it reasons from a truncated set as though it were complete.

Process fix. New rule .claude/rules/65-capped-selection-must-rank-and-disclose.md:
rank by the consumer's purpose, reuse the ranking the system already has, cap per group
where one group can dominate, disclose the truncation and how to retrieve the rest, fetch
that disclosure independently of the capped read, and give ties a total order.

A second instance of the same bug, caught in review. buildKnowledgeInstructions told
the agent the payload carried a "Full knowledge index" that "lists every entity" —
unconditionally. Whenever the index itself truncates, both halves are false. The rule-65
defect, reintroduced one layer up in prose. Fixed in c927eed56.

Review findings fixed

7 specialist reviewers ran. Beyond the two bugs above they found one real correctness
defect and four guards that could not observe the failure they existed to prevent:

• A negative limit silently disabled injection entirely. entity_rank <= -1 is
  unsatisfiable (ROW_NUMBER starts at 1), so KNOWLEDGE_AUTO_RETRIEVE_PER_ENTITY_LIMIT=-1
  would inject nothing project-wide — worse than the bug being fixed, reached by the same
  silent path. parseInt('-1') || DEFAULT does not catch it: -1 is truthy. Conversely
  SQLite reads LIMIT -1 as unbounded. Now clamped at the DO boundary so it holds for
  every caller, not just today's (rule 51).
• The per-entity cap only proved it keeps some N, never the best N. The seeding
  helper gave every observation in an entity identical confidence and age, so the window
  ORDER BY was never exercised — changing it to id ASC left the whole suite green.
• Rule-50 per-row isolation had zero coverage on both new reads, despite being the
  change's most-commented feature.
• The MAX(0, …) clock-skew clamp was untested. My first attempt at this test was
  not discriminating — at exactly +30d the unclamped denominator lands near zero and the
  score is hugely positive, ranking first either way. +90d gives a solidly negative score.
• mcp.test.ts's DO stub lacked getKnowledgeEntityIndex, so every get_instructions
  test there exercised only the degraded branch and would pass identically if the RPC were
  deleted (rule 02).

Also: folded the index total into COUNT(*) OVER (), removing a redundant JOIN+GROUP BY
per session start (rule 60); corrected an index size estimate that was ~70% low; and
softened a "cannot share an implementation" claim that was overstated (SqlStorage is
in-process — it is a transfer-size trade-off, not a language constraint).

MEDIUM/LOW deferrals tracked in tasks/backlog/2026-08-23-knowledge-injection-followups.md.

Discrimination proofs

Every guard was removed and the suite re-run — the inherited ones re-verified independently
rather than taken on trust (rule 62):

The inherited branch reported "tests passing", but the workers suite was failing to
load (unbuilt providers) and reporting "no tests" rather than a failure — the
rule-02 trap. It only became meaningful after building shared → providers → cloud-init.

Testing

• apps/api unit: 8117 passed, 0 failed, 0 collection errors (2270 files)
• apps/api workers (real DO SQLite): 706 passed, 0 failed, 0 collection errors (244 files)
• typecheck 19/19; lint 0 errors; full build clean

Totals reconciled against baseline (8115 → 8117, 695 → 706) — both moved up by exactly the
tests added, and per-file collection status was asserted, not just the failure count.

Staging Verification

Deploy 32674814563 — success. Exercised the real deployed path: seeded the production
skew through the knowledge REST API (real ProjectData DO), then called get_instructions
through the live MCP endpoint with a real MCP token. No mocks in the path.

Seed: AaaR3GrabBag (early alphabet, 20 obs @ 0.85) vs ZzzR3ContentStyle/ZzzR3UserPrefs/
ZzzR3Architecture (late alphabet, 3 each @ 0.98).

**ZzzR3Architecture** (context): …3 observations…
**ZzzR3UserPrefs** (preference): …3 observations…
**ZzzR3ContentStyle** (preference): …3 observations…
**AaaR3GrabBag** (context): …observations 20,19,18,17,16,15,14,13…   ← exactly 8 of 20

### Full knowledge index (4 entities)
AaaR3GrabBag (context, 20), ZzzR3Architecture (context, 3),
ZzzR3ContentStyle (preference, 3), ZzzR3UserPrefs (preference, 3)

All three Zzz* entities rank above the grab-bag — pre-fix ordering guarantees the
exact opposite. The cap held at 8/20. The index disclosed all 20 while showing 8. Three
identical calls returned byte-identical output. Payload keys are exactly
context, instructions, knowledgeDirectives, project, session — R1's dedup intact.

Regression: /health, app.sammy.party, /api/projects, /api/nodes, /api/workspaces,
/api/auth/me, project-scoped /tasks and /knowledge all 200; MCP tools/list exposes
113 tools. Real-browser pass against staging (authenticated via token-login): dashboard,
projects, settings all render, no horizontal overflow, 0 console errors.

All 9 seeded entities deleted afterwards; both projects verified back to pre-test state.

Agent Preflight (Required)

• Preflight completed before code changes

Classification

• external-api-change
• cross-component-change
• business-logic-change
• public-surface-change
• docs-sync-change
• security-sensitive-change
• ui-change
• infra-change

External References

.claude/rules/65-capped-selection-must-rank-and-disclose.md (added by this PR), plus rules
02 (green count is not a green suite), 24/59 (one implementation per operation), 28 (SQL
predicates need a real SQL engine), 50 (row fault isolation), 51 (server-verified values),
60 (request I/O budget) and 62 (tests must observe the real trigger). Also the R1 task record
tasks/archive/2026-08-23-remove-duplicated-structured-arrays-get-instructions.md (PR #1891),
the token-optimization research (SAM library /engineering/research/token-optimization-research.md,
§3.2 and §8/R3), and both prior attempts' branches. SQLite window-function and
negative-LIMIT semantics were verified empirically against the real Durable Object SQLite
engine rather than taken from documentation.

Codebase Impact Analysis

• apps/api/src/durable-objects/project-data/knowledge.ts — ranking SQL, per-entity cap,
  computeRelevanceScore, clampRowLimit, new getKnowledgeEntityIndex
• apps/api/src/durable-objects/project-data/index.ts — DO RPC surface (threaded
  perEntityLimit, new index RPC)
• apps/api/src/durable-objects/project-data/row-schemas{,/knowledge}.ts — index row parser
• apps/api/src/services/project-data.ts — service wrappers mirroring both signatures
• apps/api/src/routes/mcp/instruction-tools.ts — Promise.allSettled fan-out, index
  rendering, instruction strings (hand-merged with R1)
• apps/api/src/env.ts, packages/shared/src/types/knowledge.ts — two new configurable limits
• Tests: apps/api/tests/workers/knowledge-injection-ranking.test.ts (real DO SQLite),
  tests/unit/routes/mcp-instruction-context.test.ts, mcp-instruction-payload-dedup.test.ts,
  mcp.test.ts

Consumer traced end to end: get_instructions → projectDataService → ProjectData DO →
SQL, then verified through the live staging MCP endpoint. Round-trip budget: this path goes
from 2 serial DO RPCs to 3 concurrent ones, within the read-only budget (rule 60), and the
redundant COUNT(*) scan was folded into a window function to offset the addition.

Documentation & Specs

CLAUDE.md "Recent Changes" entry added; apps/api/src/env.ts documents both new env vars
inline (matching the sibling KNOWLEDGE_* convention); new process rule
.claude/rules/65-capped-selection-must-rank-and-disclose.md. No apps/www docs describe
the old alphabetical behavior — verified by grepping the public docs, specs/ and AGENTS.md
for get_instructions / getAllHighConfidenceKnowledge / auto-retrieval language; the one
substantive description (architecture/overview.md "Agent Bootstrap Payload") stays accurate
because the index is folded into knowledgeDirectives rather than added as a new field.

Constitution & Risk Check

Principle XI (no hardcoded values): both new limits are env-configurable with defaults in
KNOWLEDGE_DEFAULTS. RELEVANCE_RECENCY_SCALE_MS is retained as a curated algorithm
constant rather than a deployment knob — it is bound into the query as a parameter, so the
JS and SQL paths cannot drift on it.

Key risk and mitigation: a misconfigured limit reaching SQL. A negative perEntityLimit
makes entity_rank <= ? unsatisfiable (injecting nothing project-wide) and SQLite reads
LIMIT -1 as unbounded — neither is caught by the parseInt(...) || DEFAULT idiom, since
-1 is truthy. Both are now clamped at the DO boundary so the guarantee holds for every
caller (rule 51), with discriminating tests.

Second risk: silent truncation. The index discloses what was dropped and its heading refuses
to claim completeness when it is itself truncated; the disclosure is fetched independently of
the ranked read, so a failed ranked query still leaves the agent a retrieval path.

Specialist Review Evidence

🤖 Generated with Claude Code